-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Issue399 #406
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
replaces all show_link,link_text with config
changed required parameters of all functions and updated the descriptions.
'displaylogo', | ||
'plotGlPixelRatio', | ||
'setBackground', | ||
'topojsonURL') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if we add another config option in plotly.js? This list can't stay in-sync.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cpsievert got this right, by not validating the config options: https://github.com/ropensci/plotly/pull/446/files
At some point, I want to make the config option part of the plot schema, where they could be use to validate user input from the python api. Before that happens though, I would much prefer not validating the config options.
Related: #399 |
@@ -137,7 +161,8 @@ def _plot_html(figure_or_data, show_link, link_text, | |||
return plotly_html_div, plotdivid, width, height | |||
|
|||
|
|||
def iplot(figure_or_data, show_link=True, link_text='Export to plot.ly', | |||
def iplot(figure_or_data, | |||
config={'showLink': True, 'linkText': 'Export to plot.ly'}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we try really hard not to break backwards incompatibility. if we remove show_link
and link_text
then our user's code could break when they upgrade plotly. in this case, we might want to do something like:
iplot(figure_or_data, show_link=True, link_text='Export to plot.ly', **config_options)
Made the changes in a new branch, and create a new pull request here: |
Changed the parameters for all the functions from taking on
linkText
andshowLink
to take onconfig
(which is a dictionary of possible configurations).